LCORE-1377 Parse index name from chunk metadata source attribute by max-svistunov · Pull Request #1300 · lightspeed-core/lightspeed-stack

max-svistunov · 2026-03-10T12:05:38Z

Description

Resolve the embedded index name in the chunk metadata "source" attribute to figure out which store a chunk came from. This is done after single-store ID mapping and vector_store_id attribute resolution have been tried.

The rag-content counterpart PR is lightspeed-core/rag-content#99

(Continuation of #1135, #1208, #1248)

Type of change

Tools used to create PR

Assisted-by: Claude Opus 4.6
Generated by: Claude Opus 4.6

Related Tickets & Documents

Related Issue # LCORE-1377
Closes # LCORE-1377

Checklist before requesting a review

I have performed a self-review of my code.
PR has passed all pre-merge test jobs.
If it is a core feature, I have added thorough tests.

Testing

Start llama-stack with faiss + sentence-transformers provider
Create a vector store via REST API
Insert a chunk with metadata: {"source": "test-index", ...} through vector_io.insert
Search the vector store using vector_stores/{id}/search.
Verify result.attributes["source"] == "test-index".

Summary by CodeRabbit

Bug Fixes
- Improved source attribution when multiple data sources are configured: results now fall back to an explicit source attribute if the primary store identifier is missing, while still preferring the primary identifier when present.
Tests
- Added unit tests covering precedence and fallback behaviors for source resolution across multiple configured stores, including cases where source is present, absent, or conflicting with mappings.

coderabbitai · 2026-03-10T12:06:01Z

Walkthrough

refine resolve_source_for_result to read result.attributes["source"] as a fallback when vector_store_id is absent in multi-store setups; if source is present it is returned directly (no mapping). Added four unit tests covering precedence and edge cases.

Changes

Cohort / File(s)	Summary
Source resolution logic `src/utils/responses.py`	When multiple vector stores are configured, `_resolve_source_for_result` now falls back to `result.attributes["source"]` if `vector_store_id` is missing and returns that value directly without applying `rag_id_mapping`.
Unit tests for fallback and precedence `tests/unit/utils/test_responses.py`	Added four tests: `test_multiple_stores_source_attribute_fallback`, `test_multiple_stores_source_attribute_ignores_mapping`, `test_multiple_stores_vector_store_id_preferred_over_source`, and `test_multiple_stores_no_vector_store_id_no_source` to validate fallback behavior, mapping precedence, and absence cases.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and specifically describes the main feature added: parsing an index name from the chunk metadata source attribute.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/utils/responses.py`:
- Around line 849-852: The fallback uses attributes.get("source") but then looks
up rag_id_mapping (built as vector_db_id -> rag_id), which is the wrong key
shape and will usually return the raw metadata; change the fallback to return
attr_source directly instead of rag_id_mapping.get(attr_source, attr_source),
i.e., when attributes.get("source") is present just return attr_source, or
alternatively implement and call a dedicated index-name-to-rag lookup (e.g., an
indexNameToRag map) before this helper if you need translation—update references
to rag_id_mapping, attributes.get("source"), and the fallback branch
accordingly.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 50c19659-1ea1-42fa-b276-7ea4b3b584f9

📥 Commits

Reviewing files that changed from the base of the PR and between 36c2ae2 and bbccaa0.

📒 Files selected for processing (2)

src/utils/responses.py
tests/unit/utils/test_responses.py

src/utils/responses.py

max-svistunov · 2026-03-10T12:30:40Z

@are-ces Could you PTAL?

tisnik

LGTM

tisnik · 2026-03-10T16:01:52Z

@coderabbitai review

coderabbitai · 2026-03-10T16:02:06Z

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/utils/responses.py (1)

844-854: ⚠️ Potential issue | 🟡 Minor

Support dict-shaped results in this fallback.

parse_referenced_documents() already handles both object and dict results, but this branch still pulls attributes via getattr(...) only. In a multi-store response backed by dicts, both vector_store_id and the new source fallback will be skipped and source resolves to None.

🩹 Proposed fix

     if len(vector_store_ids) > 1:
-        attributes = getattr(result, "attributes", {}) or {}
+        if isinstance(result, dict):
+            attributes = result.get("attributes", {}) or {}
+        else:
+            attributes = getattr(result, "attributes", {}) or {}
         attr_store_id: Optional[str] = attributes.get("vector_store_id")
         if attr_store_id:
             return rag_id_mapping.get(attr_store_id, attr_store_id)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/utils/responses.py` around lines 844 - 854, The fallback assumes result
is an object and uses getattr(result, "attributes", {}), which skips dict-shaped
results; update the attributes extraction in parse_referenced_documents() (the
block that sets attributes, attr_store_id, attr_source and uses rag_id_mapping)
to first handle dictionaries (e.g., if isinstance(result, dict): attributes =
result.get("attributes", {}) ) and otherwise use getattr, then proceed to check
attr_store_id and attr_source as before so both "vector_store_id" and the
"source" fallback work for dict and object results.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/utils/responses.py`:
- Around line 844-854: The fallback assumes result is an object and uses
getattr(result, "attributes", {}), which skips dict-shaped results; update the
attributes extraction in parse_referenced_documents() (the block that sets
attributes, attr_store_id, attr_source and uses rag_id_mapping) to first handle
dictionaries (e.g., if isinstance(result, dict): attributes =
result.get("attributes", {}) ) and otherwise use getattr, then proceed to check
attr_store_id and attr_source as before so both "vector_store_id" and the
"source" fallback work for dict and object results.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 6c64e5a7-232b-43e1-ae13-1e82277a704a

📥 Commits

Reviewing files that changed from the base of the PR and between bbccaa0 and 3c1abde.

📒 Files selected for processing (2)

src/utils/responses.py
tests/unit/utils/test_responses.py

🚧 Files skipped from review as they are similar to previous changes (1)

tests/unit/utils/test_responses.py

max-svistunov · 2026-03-10T18:37:29Z

Thanks @tisnik , could you also PTAL at the rag-content counterpart?

lightspeed-core/rag-content#99

are-ces

Just a nit, otherwise LGTM

src/utils/responses.py

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/utils/responses.py`:
- Around line 844-850: The branch in parse_referenced_documents() uses
getattr(result, "attributes", {}) which misses dict-shaped results; change
attribute extraction to handle both objects and dicts (e.g., if
isinstance(result, dict): attributes = result.get("attributes", {}) else:
attributes = getattr(result, "attributes", {}) or {}), and when resolving
attr_source use attributes.get("source") with a fallback to result.get("source")
for dict-backed results so dict results can expose the embedded "source" the
same way object results do.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 57799d26-976e-41b2-9344-c2472d3afe91

📥 Commits

Reviewing files that changed from the base of the PR and between 3c1abde and 5fb28b9.

📒 Files selected for processing (2)

src/utils/responses.py
tests/unit/utils/test_responses.py

🚧 Files skipped from review as they are similar to previous changes (1)

tests/unit/utils/test_responses.py

coderabbitai · 2026-03-11T13:06:56Z

src/utils/responses.py

        attributes = getattr(result, "attributes", {}) or {}
+
+        # Primary: read index name embedded directly by rag-content.
+        # This value is already the user-facing rag_id, not a vector_db_id,
+        # so no mapping is needed.
+        attr_source: Optional[str] = attributes.get("source")
+        if attr_source:


⚠️ Potential issue | 🟠 Major

Handle dict-backed results before reading source.

parse_referenced_documents() already supports both object and dict-shaped results, but this branch only uses getattr. When result is a dict, Line 849 never sees the embedded source, so multi-store source resolution falls back to None for referenced documents.

Suggested fix

- attributes = getattr(result, "attributes", {}) or {} + raw_attributes = ( + result.get("attributes", {}) + if isinstance(result, Mapping) + else getattr(result, "attributes", {}) + ) + attributes = raw_attributes if isinstance(raw_attributes, Mapping) else {}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@src/utils/responses.py` around lines 844 - 850, The branch in parse_referenced_documents() uses getattr(result, "attributes", {}) which misses dict-shaped results; change attribute extraction to handle both objects and dicts (e.g., if isinstance(result, dict): attributes = result.get("attributes", {}) else: attributes = getattr(result, "attributes", {}) or {}), and when resolving attr_source use attributes.get("source") with a fallback to result.get("source") for dict-backed results so dict results can expose the embedded "source" the same way object results do.

are-ces

LGTM

max-svistunov mentioned this pull request Mar 10, 2026

LCORE-1377 Embed index name as source attribute in chunk metadata lightspeed-core/rag-content#99

Merged

19 tasks

coderabbitai bot reviewed Mar 10, 2026

View reviewed changes

src/utils/responses.py Outdated Show resolved Hide resolved

max-svistunov force-pushed the lcore-1377-parse-index-name-from-chunk-metadata branch from bbccaa0 to 3c1abde Compare March 10, 2026 12:29

tisnik approved these changes Mar 10, 2026

View reviewed changes

coderabbitai bot reviewed Mar 10, 2026

View reviewed changes

tisnik requested a review from are-ces March 10, 2026 16:21

are-ces requested changes Mar 11, 2026

View reviewed changes

src/utils/responses.py Outdated Show resolved Hide resolved

Parse index name from chunk metadata source attribute

5fb28b9

max-svistunov force-pushed the lcore-1377-parse-index-name-from-chunk-metadata branch from 3c1abde to 5fb28b9 Compare March 11, 2026 12:59

coderabbitai bot reviewed Mar 11, 2026

View reviewed changes

are-ces approved these changes Mar 11, 2026

View reviewed changes

tisnik merged commit d238a82 into lightspeed-core:main Mar 11, 2026
23 checks passed

Conversation

max-svistunov commented Mar 10, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

Tools used to create PR

Related Tickets & Documents

Checklist before requesting a review

Testing

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

max-svistunov commented Mar 10, 2026

Uh oh!

tisnik left a comment

Choose a reason for hiding this comment

Uh oh!

tisnik commented Mar 10, 2026

Uh oh!

coderabbitai bot commented Mar 10, 2026

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

max-svistunov commented Mar 10, 2026

Uh oh!

are-ces left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

are-ces left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

max-svistunov commented Mar 10, 2026 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Mar 10, 2026 •

edited

Loading